chore: WS-8 follow-ups — parity audit fix, README caveat, Dependabot auto-merge, locale key dedup - #281
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Dependabot semver-patch auto-merge automation, updates localized backup and settings strings, and enhances feature parity auditing to recognize dedicated settings toggles. ChangesDependabot automation
Localization updates
Feature parity auditing
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
96a888c to
9c1bebb
Compare
c3dd458 to
7995680
Compare
9c1bebb to
aadad0e
Compare
7995680 to
2bad608
Compare
aadad0e to
e48ffcb
Compare
…module files The new docs:check drift gate (this same branch) immediately caught a real bug on its first CI run: README claimed 2859 i18n keys while the actual deduplicated count is 2849. Root cause is in sync-readme-metrics.mjs itself, not just the (already-fixed-elsewhere) duplicate source keys — getKeyCount() summed leaf counts per module file independently, so any key defined in two files (as 10 keys were, until the PR #281 dedup fix) got counted twice. This is a latent bug: it will silently drift again the moment any future PR reintroduces a cross-module duplicate, regardless of today's specific fix. Changed getKeyCount() to collect keys into a Set across all module files, matching check-i18n-keys.mjs's loadBundleKeys() dedup logic exactly — the two scripts can no longer disagree about the canonical key count.
…ce (#283) * chore(repo): fix README badges, add Code of Conduct, link it from both CONTRIBUTING files - README.md: removed the duplicate codecov badge (kept the more informative `&label=Coverage` one) and the CII Best Practices badge, which had a literal `projects/XXXX` placeholder (broken image, dead link) — the project was never registered. Added a TODO.md entry with the registration URL; that's an external, interactive signup, so it stays a maintainer action, not something this pass does automatically. - Added .github/CODE_OF_CONDUCT.md (Contributor Covenant 2.1), with an enforcement-contact section that mirrors SECURITY.md's pattern (GitHub Private Vulnerability Reporting, no hardcoded personal email). - .github/CONTRIBUTING.md already referenced a Code of Conduct but linked externally to contributor-covenant.org instead of the new in-repo file — repointed it. - The root CONTRIBUTING.md — the one README.md actually links to — had no Code of Conduct section at all. Added one, linking to the new file. Investigated (no code changes, per the workstream's stop-and-ask boundaries): - The v1.5.0 GitHub release sits chronologically between v1.20.0 and v1.21.0 despite its lower semver, AND its 6 assets are all named "StoryCraft.Studio_1.20.0_..." — the project's pre-rebrand name (rebrand to WorldScript Studio shipped in v1.23.0) with a version that doesn't match the release tag at all. Flagged for the final report; deleting/retitling a public release is a maintainer decision. - Issue #60 (vendor-fork audit, "permanent reminder — do not close") already has a 2026-07-28 maintainer comment documenting the audit as done, and `pnpm run verify:vendor` passes all 12 invariants. No action needed — it's correctly staying open per its own instruction. * chore(docs): truth-up locale counts + release status, add doc-metrics drift gate ROADMAP.md still marked v1.24 as "(PLANNED)" though v1.24.0 (2026-06-21) and v1.24.1 (2026-07-28) had both already shipped, and gave the locale count as 17 — stale since the 11->17 expansion (v1.24.0) plus ru/ko (landed the next day, before v1.24.1). Same "17 locales" drift was independently present in docs/TRANSLATION-GUIDE.md (also missing 1 of 21 modules — desktop.json — from its module list and count), the root CONTRIBUTING.md (the one README.md actually links to), .github/copilot-instructions.md (2 occurrences), and docs/i18n/AUDIT_AND_IMPROVEMENT_PLAN.md's "current state" section. Fixed all of them; left every CHANGELOG.md and dated AUDIT.md/TODO.md historical entry untouched — they correctly describe what was true at a past release. Added scripts/check-doc-metrics.mjs (wired into ci.yml's quality job as `pnpm run docs:check`) so this class of drift fails CI instead of silently accumulating again. It computes the actual locale count, key count (dedup'd the same way check-i18n-keys.mjs does), and latest released version tag, then scans README/ROADMAP/TODO/TRANSLATION-GUIDE/CONTRIBUTING for stale numbers or a "vX.Y.Z ... PLANNED" claim for an already-released version. Historical sections are exempt (Keep-a-Changelog `## [x.y.z]`, `## vX.Y.Z ... RELEASED ...`, a heading suffixed with an emoji checkmark, a `**Status:** ✅ Released/Completed` line just below a heading, or this repo's own TODO.md "✅ = done" bullet convention) — the first version only recognized the Keep-a-Changelog/RELEASED-heading shapes and produced 26 false positives against TODO.md/ROADMAP.md's other historical conventions; broadened it before wiring the gate in. tests/unit/checkDocMetrics.test.ts covers the exclusion heuristic directly (the plan's explicit requirement — an untested exclusion rule turns a drift gate into noise). * chore(gates): document coverage-ratchet tooling, suppression-debt plan, mutation-testing status - Re-pinned token-audit-baseline.json to the actual measured total (160, down from a stale 165 dated 2026-06-14) after WS-1..6 landed. Ratchets down only, per the project's hard rule against loosening a quality gate. - Added scripts/check-coverage-ratchet.mjs: reads coverage/coverage-summary.json (added the `json-summary` reporter to vitest.config.ts — it wasn't being generated at all) and suggests raising a threshold when the measured value sits 2+ points above it, matching the "3 green runs, max 5 pts/quarter" rule already documented as a comment in vitest.config.ts but never mechanized. Wired into ci.yml as `continue-on-error: true` — informational only, never blocks the build; the ratchet decision stays with the maintainer. - docs/CODE_QUALITY.md: added a Suppression Reduction Plan grouping all 33 noExplicitAny suppressions (of 52 total, at an exact-match baseline with no headroom) into clusters by root cause and effort — no code changes, visibility only. - docs/CI.md: corrected three places that still described Stryker mutation testing as part of the active `ci.yml` job graph (job-graph diagram, the `mutation` job's table row, and the CI-tier table) — it was removed from the merge pipeline on 2026-06-02 and now runs only via manual `workflow_dispatch` on mutation.yml. Documented why, how to trigger it, and the re-integration criterion. * docs: resync README test-file-count badge (527→528) after rebase onto new main * fix(scripts): dedupe sync-readme-metrics.mjs's i18n key count across module files The new docs:check drift gate (this same branch) immediately caught a real bug on its first CI run: README claimed 2859 i18n keys while the actual deduplicated count is 2849. Root cause is in sync-readme-metrics.mjs itself, not just the (already-fixed-elsewhere) duplicate source keys — getKeyCount() summed leaf counts per module file independently, so any key defined in two files (as 10 keys were, until the PR #281 dedup fix) got counted twice. This is a latent bug: it will silently drift again the moment any future PR reintroduces a cross-module duplicate, regardless of today's specific fix. Changed getKeyCount() to collect keys into a Set across all module files, matching check-i18n-keys.mjs's loadBundleKeys() dedup logic exactly — the two scripts can no longer disagree about the canonical key count. * fix: address CodeRabbit review wave on PR #283 Real bugs found and fixed: - .github/CONTRIBUTING.md linked SECURITY.md as ../SECURITY.md, but that file lives at .github/SECURITY.md — a sibling of CONTRIBUTING.md, not one level up. The link resolved to a nonexistent repo-root file. CodeRabbit anchored this finding on .github/CODE_OF_CONDUCT.md, whose own (correct) sibling link I verified matches; the actual broken link was in CONTRIBUTING.md. - .github/workflows/ci.yml's quality-job checkout never fetched tags, so check-doc-metrics.mjs's stale-PLANNED-status check (which reads `git tag`) silently no-oped in every CI run. Added fetch-tags: true. - check-coverage-ratchet.mjs's JSON.parse had no try/catch — a truncated or invalid coverage-summary.json would throw uncaught, contradicting the script's own documented "always exits 0, never gates CI" contract. Now falls through to the same non-blocking skip message as a missing file. - check-doc-metrics.mjs's TARGET_FILES list didn't include .github/copilot-instructions.md or .github/CONTRIBUTING.md, despite the script's own header comment naming both as files that had the exact stale- locale-count regression this gate exists to catch — a recurrence there would have passed docs:check silently. - docs/CODE_QUALITY.md's suppression cluster table summed to ~53 against the stated 52/52 baseline. Recomputed exact per-cluster counts from `check-suppressions.mjs --details` (11 + 8 + 5 + 9 + 19 = 52) and corrected the useAppSelector/single-occurrence cluster file lists to match. - README.md's project-tree comment still said "15 modules"; the doc-truth-up work elsewhere in this PR already established 19 locales × 21 modules. - docs/CI.md's pipeline table said `tsc` where the actual quality-job step (and package.json) both use `pnpm run typecheck`. Convention-only: - Added the missing single-line QNBS-v3 rationale comments to check-doc-metrics.mjs (filesystem reads, tag lookup, drift scan, CLI orchestration) and check-coverage-ratchet.mjs (non-blocking contract). - Collapsed checkDocMetrics.test.ts's multiline file-header rationale to one line.
… parity audit audit-feature-parity.ts's "has a UI toggle" check only looked at the generic FeatureFlagsSection.tsx catalog rendering, so enableIdbAtRestEncryption — deliberately hidden from that generic list because toggling it without the passphrase setup flow would lock users out — was reported as "no UI toggle" even though it has its own dedicated one in PrivacySection.tsx. Added extractDedicatedUiFlags(), which reads each catalog entry's existing gateLocations field (already documents "components/settings/PrivacySection.tsx:20") and treats a gateLocation under components/settings/ (other than FeatureFlagsSection.tsx itself) as a real UI toggle. The audit now reports this as informational (shown distinctly as ◆ in the table) instead of a warning; 0 warnings instead of 1.
A 31-day commit gap (2026-06-25 to 2026-07-26) let 20 Dependabot PRs stack up (see AUDIT.md / TODO.md), all triaged in one burst afterward. Auto-merging patch-level bumps once CI is green cuts that latency without touching the judgment calls minor/major bumps deserve — those still land as normal PRs. Uses dependabot/fetch-metadata (pinned to its v3.1.0 commit) to read the update-type, then `gh pr merge --auto --squash` only for version-update:semver-patch. The existing dev-tooling/tauri-deps groups and the 7-day release cooldown (.npmrc minimum-release-age) are unchanged.
…n/dashboard.json, fix ES mistranslation
Found while investigating why scripts/sync-readme-metrics.mjs's i18n key
count kept disagreeing with the canonical check-i18n-keys.mjs count by
exactly 10: sceneboard.minimap.ariaLabel and both error.boundary.* keys were
defined in BOTH common.json and settings.json; all 7 dashboard.backup.* keys
were defined in BOTH dashboard.json and settings.json — in every locale.
Since the runtime bundle is a flat merge across all 21 modules, a duplicate
key isn't just redundant storage — whichever module the merge processes last
silently wins, so the two copies could (and did) diverge per locale without
anything catching it. Removed the settings.json copy in all 19 locales,
keeping each key in its semantic "home" module (common.json for the
cross-cutting error/sceneboard keys, dashboard.json for the backup ones).
While auditing which copy was actually correct per locale: **es's
dashboard.json copies of all 7 dashboard.backup.* keys were in German**
("Backup & Wiederherstellung", "Projekt exportieren...") — not Spanish —
while the settings.json duplicates were untranslated English placeholders.
Neither was Spanish. Replaced with real translations matching this locale's
established terminology elsewhere in the app (help.json: "copia de
seguridad", "instantánea").
Every other locale's two copies were either identical or both legitimate
(alternate phrasing); a few had a mixed-language artifact in the settings.json
copy specifically (e.g. ja/pt/zh: "エクスポート your project, import a backup"),
which is now gone since that copy no longer exists.
i18n:check confirms the deduplicated key count is unchanged (2849 — the
duplicates were never counted as extra keys by the canonical dedup-via-Set
logic, only by sync-readme-metrics.mjs's naive per-file sum). That script now
settles on the correct 2849 on every run instead of flip-flopping to 2859.
2bad608 to
5f01793
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Action performedReview finished.
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/dependabot-automerge.yml (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep operational rationale out of this pure YAML file.
The newly added explanatory blocks at Lines [1] through [5] and Lines [30] through [31] are PR history rather than workflow configuration. Move the rationale and impact to the commit or PR description instead.
As per coding guidelines, pure YAML changes must not add forced inline comments; document the rationale in the commit description or response.
Also applies to: 30-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/dependabot-automerge.yml around lines 1 - 5, Remove the explanatory PR-history comments from the workflow, including the blocks at the file header and around the added merge step. Keep only valid YAML configuration; preserve the existing Dependabot grouping, release cooldown, and CI/patch-only merge behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/dependabot-automerge.yml:
- Around line 18-22: Update the workflow job condition using the visible
github.actor check to inspect github.event.pull_request.user.login instead,
while preserving the existing repository head check. Ensure the job granting
contents: write and pull-requests: write permissions runs only for PRs authored
by Dependabot.
In `@scripts/audit-feature-parity.ts`:
- Around line 137-142: Add a single-line `// QNBS-v3: [...]` marker immediately
adjacent to every substantive changed block in the audit script, including the
row-model, warning-count, and reporting changes and the additional listed
blocks. Replace any existing multi-line markers with the required single-line
format, preserving accurate descriptions of each change and ensuring no changed
block lacks a marker.
---
Nitpick comments:
In @.github/workflows/dependabot-automerge.yml:
- Around line 1-5: Remove the explanatory PR-history comments from the workflow,
including the blocks at the file header and around the added merge step. Keep
only valid YAML configuration; preserve the existing Dependabot grouping,
release cooldown, and CI/patch-only merge behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27a19f46-06b3-4e21-8390-6a51d66756a0
📒 Files selected for processing (34)
.github/workflows/dependabot-automerge.ymllocales/ar/settings.jsonlocales/de/settings.jsonlocales/el/settings.jsonlocales/en/settings.jsonlocales/es/dashboard.jsonlocales/es/settings.jsonlocales/eu/settings.jsonlocales/fa/settings.jsonlocales/fi/settings.jsonlocales/fr/settings.jsonlocales/he/settings.jsonlocales/hu/settings.jsonlocales/is/settings.jsonlocales/it/settings.jsonlocales/ja/settings.jsonlocales/ko/settings.jsonlocales/pt/settings.jsonlocales/ru/settings.jsonlocales/sv/settings.jsonlocales/zh/settings.jsonpublic/locales/el/bundle.jsonpublic/locales/en/bundle.jsonpublic/locales/es/bundle.jsonpublic/locales/eu/bundle.jsonpublic/locales/fa/bundle.jsonpublic/locales/fi/bundle.jsonpublic/locales/hu/bundle.jsonpublic/locales/is/bundle.jsonpublic/locales/ja/bundle.jsonpublic/locales/pt/bundle.jsonpublic/locales/sv/bundle.jsonpublic/locales/zh/bundle.jsonscripts/audit-feature-parity.ts
💤 Files with no reviewable changes (18)
- locales/hu/settings.json
- locales/fi/settings.json
- locales/fr/settings.json
- locales/zh/settings.json
- locales/es/settings.json
- locales/de/settings.json
- locales/ja/settings.json
- locales/is/settings.json
- locales/el/settings.json
- locales/it/settings.json
- locales/eu/settings.json
- locales/he/settings.json
- locales/sv/settings.json
- locales/fa/settings.json
- locales/ru/settings.json
- locales/pt/settings.json
- locales/ar/settings.json
- locales/ko/settings.json
- dependabot-automerge.yml gated a contents:write/pull-requests:write job on github.actor == 'dependabot[bot]'. github.actor reflects the last actor to trigger the workflow event, not the immutable PR author, and is spoofable (confused-deputy risk — zizmor's bot-conditions rule flagged this too). Switched to github.event.pull_request.user.login, set at PR creation and unaffected by later synchronize/re-run events. - audit-feature-parity.ts: collapsed a 6-line rationale comment to one line and added the single-line QNBS-v3 markers CodeRabbit found missing on the dedicatedUiFlags row-model/warning-count/reporting changes.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
Fourth stacked PR in the deployment-surface/OpenRouter/doc-gates audit sequence (base: #280
chore/release-doc-gates). Bundles the WS-8 investigation follow-ups that turned into concrete, low-risk fixes, plus a root-cause i18n fix discovered while re-verifying README's auto-synced metrics.fix(feature-flags)—scripts/audit-feature-parity.tsnow reads each flag'sgateLocationsentry to recognize a dedicated per-feature UI toggle living outsideFeatureFlagsSection.tsx(e.g.enableIdbAtRestEncryption→PrivacySection.tsx). Resolves the one outstanding parity-audit warning without hiding it —pnpm run parity:checknow reports 0 warnings.docs— discloses in README that Tauri macOS builds are arm64-only (macos-latestrunner;macos-13was dropped from the matrix because it's no longer provisioned by GitHub — see the prior CI-matrix fix).ci— adds.github/workflows/dependabot-automerge.yml: auto-merges Dependabot PRs only whenupdate-type == version-update:semver-patchand CI is green (gh pr merge --auto --squash), gated ongithub.actor == 'dependabot[bot]'. Minor/major bumps and grouped/mixed-level updates still require a human. Action pinned to full commit SHA (dependabot/fetch-metadata@25dd0e34...# v3.1.0), top-levelpermissions: contents: read, job-level write permissions scoped to this job only.fix(i18n)— deduplicates 10 keys that existed in bothsettings.jsonand their semantic home module (common.json/dashboard.json) across all 19 locales. This is whyscripts/sync-readme-metrics.mjs's naive per-file leaf-count kept disagreeing withcheck-i18n-keys.mjs's canonical Set-deduplicated count by exactly 10, flip-flopping the README key-count badge. While auditing which copy was authoritative per locale, foundes/dashboard.json'sdashboard.backup.*values were actually German, withes/settings.json's duplicate copies left in English — neither was Spanish. Replaced with real Spanish translations matching this locale's existing terminology (help.json: "copia de seguridad", "instantánea").Why these are bundled together
None of these are large enough to warrant their own PR, none touch overlapping files, and all fall out directly of the WS-8 investigation / final-verification pass from the parent audit — bundling avoids four near-empty PRs.
Test plan
pnpm run parity:check→ 0 warnings, 0 critical errors (was 1 warning)pnpm run docs:check→ OK, 19 locales / 2849 keys / latest v1.24.1 (stable across repeated runs now)pnpm run token:audit→ PASSpnpm run i18n:check→ key parity holds, dedup didn't change the canonical count (2849)pnpm run lint/pnpm run typecheck→ cleanpnpm run build && pnpm run bundle:budget→ OK, 154 JS chunks ≤ 6200 KBBackupQuickActionsCard,ViewErrorBoundary,creativityTemperature) → 14/14 passmain)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores